Improve types and bytes usage of pathlib#9016
Conversation
| def read_text(self, encoding: str | None = ..., errors: str | None = ...) -> str: ... | ||
| def samefile(self, other_path: str | bytes | int | Path) -> bool: ... | ||
| def write_bytes(self, data: bytes) -> int: ... | ||
| def samefile(self, other_path: StrPath) -> bool: ... |
There was a problem hiding this comment.
This just calls other_path.stat(), which should also work with Path[bytes]. In case it's not a Path, it's passed on to os.stat(), which uses int | StrOrBytesPath. I therefore think the following would be correct:
| def samefile(self, other_path: StrPath) -> bool: ... | |
| def samefile(self, other_path: StrOrBytesPath | int) -> bool: ... |
There was a problem hiding this comment.
Path[bytes] isn't a thing — Path isn't a generic class. pathlib only makes promises to work with string paths, unlike os.path or functions in os
There was a problem hiding this comment.
I always mix up Path and PathLike. The annotation should be correct nevertheless.
There was a problem hiding this comment.
Here' how this method is implemented:
def samefile(self, other_path):
st = self.stat()
try:
other_st = other_path.stat()
except AttributeError:
other_st = self.__class__(other_path).stat()
return os.path.samestat(st, other_st)So, other_path can either be:
- Something with
.stat()method (assumingPath, it can be a new protocol, but I don't think it is worth it right now) - Something that
self.__class__.__new__accepts:StrPath(==str | PathLike[str])
But, Path is a part of StrPath, because Path is PathLike[str].
That's why StrPath seems correct to me. Am I missing something?
There was a problem hiding this comment.
Ah interesting, I was looking at the 3.9 version of the implementation:
def samefile(self, other_path):
"""Return whether other_path is the same or not as this file
(as returned by os.path.samefile()).
"""
st = self.stat()
try:
other_st = other_path.stat()
except AttributeError:
other_st = self._accessor.stat(other_path)
return os.path.samestat(st, other_st)No need to version guard this. Let's just use the latest version (yours).
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: bandersnatch (https://github.com/pypa/bandersnatch)
+ src/bandersnatch_storage_plugins/swift.py: note: In member "write_bytes" of class "SwiftPath":
+ src/bandersnatch_storage_plugins/swift.py:408: error: Signature of "write_bytes" incompatible with supertype "Path"
+ src/bandersnatch_storage_plugins/swift.py:408: note: Superclass:
+ src/bandersnatch_storage_plugins/swift.py:408: note: def write_bytes(self, data: Union[bytes, Union[bytearray, memoryview, array[Any], mmap, _CData, PickleBuffer]]) -> int
+ src/bandersnatch_storage_plugins/swift.py:408: note: Subclass:
+ src/bandersnatch_storage_plugins/swift.py:408: note: def write_bytes(self, contents: bytes, encoding: Optional[str] = ..., errors: Optional[str] = ...) -> int
|
3 participants
Notes:
link_tois removed in 3.12: https://github.com/python/cpython/blob/3.11/Lib/pathlib.py#L1222-L1225samefileuses the same args asPath.__new__Source: https://github.com/python/cpython/blob/main/Lib/pathlib.py
Refs #9006